Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use IBuildDependencyProjectReferencesService to read projects references for CPS based packages.config projects #4205

Merged
merged 6 commits into from
Sep 1, 2021

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented Aug 17, 2021

Bug

Fixes: NuGet/Home#11162

Regression? Last working version:

Description

There's a restore time UI delay:

UIDelay: nuget.packagemanagement.visualstudio.dll!NuGet.PackageManagement.VisualStudio.VsCoreProjectSystemReferenceReader+d__

It happens when mixed solutions are used, ones that have PackageReference projects & legacy managed projects & native projects.

The walk that we run in

var itemsFactory = ServiceLocator.GetInstance<IVsEnumHierarchyItemsFactory>();
is inefficient and in some cases, such as the native case, it might even trigger a design time build.

The fix is to use a CPS specific API which is free threaded.
Note that there's currently a bug with this API: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1374627, which is being fixed as we speak.

Second part of this UI delay will be fixed in NuGet/Home#11163.

cc @lifengl @davkean

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@nkolev92 nkolev92 force-pushed the dev-nkolev92-addCpsProjectSystemServices branch from 4104470 to c735a9d Compare August 17, 2021 21:03
@nkolev92 nkolev92 marked this pull request as ready for review August 18, 2021 02:09
@nkolev92 nkolev92 requested a review from a team as a code owner August 18, 2021 02:09
@nkolev92 nkolev92 force-pushed the dev-nkolev92-addCpsProjectSystemServices branch from f8c16fb to 01997dd Compare August 18, 2021 05:12
@nkolev92 nkolev92 force-pushed the dev-nkolev92-addCpsProjectSystemServices branch 2 times, most recently from 9d6168c to 4c608a9 Compare August 25, 2021 18:13
@nkolev92
Copy link
Member Author

@davkean

Can you please take another look? Thanks!

@NuGet/nuget-client I need a review from you as well.

Copy link
Contributor

@davkean davkean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that await null is fixed, this looks good.

@nkolev92 nkolev92 force-pushed the dev-nkolev92-addCpsProjectSystemServices branch from 4c608a9 to 07ecf25 Compare August 31, 2021 07:45
@nkolev92
Copy link
Member Author

@NuGet/nuget-client Can someone take a look please?🔔

@nkolev92 nkolev92 merged commit a8ff38e into dev Sep 1, 2021
@nkolev92 nkolev92 deleted the dev-nkolev92-addCpsProjectSystemServices branch September 1, 2021 01:58
kartheekp-ms pushed a commit that referenced this pull request Sep 2, 2021
… references for CPS based packages.config projects (#4205)"

This reverts commit a8ff38e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants